The release ZIP for this PR is accessible via:

https://wcblocks.wpcomstaging.com/wp-content/uploads/woocommerce-gutenberg-products-block-11120.zip

Script Dependencies Report

There is no changed script dependency between this branch and trunk.

This comment was automatically generated by the ./github/compare-assets action.

TypeScript Errors Report

• Files with errors: 523
• Total errors: 2335

🎉 🎉 This PR does not introduce new TS errors.

_{comments-aggregator}

Size Change: +172 B (0%)

Total Size: 1.54 MB

_{compressed-size-action}

Thanks for working on this issue! I noticed that there is a style issue on the editor side on small screens:
Screen.Capture.Oct.10.mov

I am not sure about this, but it seems that this PR introduces some regression on the front side:
Screen.Capture.Oct.10.mp4

@gigitux Could you please elaborate further on what you mean by the style issue and the regression?

Please also note from the description of the PR:

This PR does not fix the issue of the Pager overflowing the Dialog. We probably should enable Margin control for it and add the fix via that method.

Sorry for not being clear, I was convinced that the videos were self-explanatory.

On the editor side, the arrows disappear on "large window" and appear when the window is smaller. I think that this should be addressed.

On the frontend side, during the zoom effect, it seems that the image moves. I think that this is a regression introduced by this PR. (I'm not able to replicate it on trunk)

Thank you for clarifying, @gigitux!

On the editor side, the arrows disappear on "large window" and appear when the window is smaller. I think that this should be addressed.

Well, that's kind of to be expected. We can reduce the height of the Large Image in the editor, but then the placeholder will not extend to the full width of the container.

On the frontend side, during the zoom effect, it seems that the image moves. I think that this is a regression introduced by this PR. (I'm not able to replicate it on trunk)

Do we want/need zoom in the Dialog? That behavior is to be expected if the image isn't wide enough to fill out the entire width. I'm not sure how we can "fix" this. Do you have any suggestions?

Thank you for clarifying, @gigitux!

On the editor side, the arrows disappear on "large window" and appear when the window is smaller. I think that this should be addressed.

Well, that's kind of to be expected. We can reduce the height of the Large Image in the editor, but then the placeholder will not extend to the full width of the container.

Aaah ok! I didn't realize that there was a scrollbar. Could it sense to make the image smaller?

On the frontend side, during the zoom effect, it seems that the image moves. I think that this is a regression introduced by this PR. (I'm not able to replicate it on trunk)

Do we want/need zoom in the Dialog? That behavior is to be expected if the image isn't wide enough to fill out the entire width. I'm not sure how we can "fix" this. Do you have any suggestions?

Since that we are using the same block in the dialog, we should support all the functionalities in any context. The only thing that we can do, is provide a default state that doesn't have the zoom effect enabled.

The changes that I did in #11113 could help you.

Aaah ok! I didn't realize that there was a scrollbar. Could it sense to make the image smaller?

Would it be beneficial if it would fit in the viewport: yes. The question is how? It's set to Full-Width because that's how it's supposed to display in the front end. I don't know what a potential solution could be here.

Since that we are using the same block in the dialog, we should support all the functionalities in any context. The only thing that we can do, is provide a default state that doesn't have the zoom effect enabled.

I agree in principle (even though, for example, Full-screen when clicked doesn't do anything in the Dialog).

It will be very tricky to "fix", as the way it behaves now is technically correct. The problem is that the image isn't wide enough, so when Zoomed it's a wonky zoom.

I'll have an additional look, next week.

It will be very tricky to "fix", as the way it behaves now is technically correct. The problem is that the image isn't wide enough, so when Zoomed it's a wonky zoom.

I'll have an additional look, next week.

I was, unfortunately, unable to find a solution to this.

For this not to be a blocker, I'd suggest disabling Zoom in the context of the Dialog altogether.

cc: @gigitux thoughts?

For this not to be a blocker, I'd suggest disabling Zoom in the context of the Dialog altogether.

Did you try to pull trunk? Did we fix it with #11113?

This PR has been marked as stale because it has not seen any activity within the past 7 days. Our team uses this tool to help surface pull requests that have slipped through review.

If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label.

This PR is no longer relevant.